-
Notifications
You must be signed in to change notification settings - Fork 119
Rust: Update build features #804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… agnosticism Added missing features to BuildFeatures struct and implemented conditional defines for correct handling across cc and cmake build systems.
…erf regression Ensures assertions and expensive checks are disabled in release builds when using build_cc, addressing issue microsoft#202.
Enables WaitOnAddress support (using Futex) for build_cc on Linux, resolving the spinning behavior issue. Also defines SNMALLOC_HAS_LINUX_RANDOM_H and SNMALLOC_PLATFORM_HAS_GETENTROPY which are standard on modern Linux.
…ild_cc only Reverts NDEBUG change that broke cmake builds. Also guards SNMALLOC_HAS_LINUX_FUTEX_H and related definitions with #[cfg(feature = 'build_cc')] to prevent build errors when using cmake (default).
Explicitly sets WINVER/_WIN32_WINNT to 0x0A00 (Win10) by default to enable VirtualAlloc2 and WaitOnAddress. Sets to 0x0603 (Win8.1) if win8compat feature is enabled. Matches snmalloc CMake behavior and ensures WaitOnAddress is available.
Adds 'usewait-on-address' to default features in Cargo.toml. This aligns the Rust build behavior with upstream CMake defaults, where SNMALLOC_ENABLE_WAIT_ON_ADDRESS is ON by default. Prevents silent performance regression (spinning vs futex) for users relying on default features.
Add a new workspace member `xtask` containing a CLI tool to compare performance between the `build_cc` and `build_cmake` backends. The tool automates building, running a contention benchmark, and parsing throughput results to verify performance equivalence. Also includes the new benchmark example `bench_contention` which allocates and deallocates memory across a ring of threads to stress the allocator.
Updates build.rs to use compiler flags (-D) instead of CMake variables for WINVER and _WIN32_WINNT when using the cmake builder. This ensures these critical definitions reach the snmalloc C++ compiler, enabling WaitOnAddress (Futex) support on Windows and resolving a performance discrepancy where build_cmake was ~20% slower than build_cc.
Refactor the platform-specific configuration in build.rs by replacing match blocks with if/else statements for better clarity. This change consolidates Windows and Unix flag handling, fixes MSVC flag definitions, and adds specific support for FreeBSD.
Update the build script to honor CARGO_CFG_TARGET_OS and OPT_LEVEL environment variables. This ensures that snmalloc is compiled with settings consistent with the rest of the Rust project, rather than relying solely on the "debug" feature flag.
|
@ryancinsight if you have a chance, would you be able to check I have not messed up the merge. Thanks |
| - `debug`: Enable the `Debug` mode in `snmalloc`. This is also automatically enabled if Cargo's `DEBUG` environment variable is set to `true`. | ||
| - `native-cpu`: Optimize `snmalloc` for the native CPU of the host machine. (this is not a default behavior |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SchrodingerZhu did you have a reason to separate snmalloc debug from rust compile, I modified this yesterday base on @mjp41 feedback but seeing this flag in readme is making me question if you had a reason
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At that time letting cmake-rs decide debug build was not that prevalent. I agree it is more preferable to have a unified debug build now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to simplify the options. As we're about to release with a big version number change, Now is a good time to put a breaking change in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to simplify the options. As we're about to release with a big version number change, Now is a good time to put a breaking change in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose we take this as is, and then if someone could simplify the options in a subsequent PR that would be great.
|
left one comment, otherwise looks good, you may not need the xtask or example, they are kind of simple benchmarks that I was using locally to validate and compare performance of builds |
|
Also LGTM |
This is @ryancinsight's PR from snmalloc-rs replayed on the post-merged snmalloc and snmalloc-rs.
This pull request updates the feature configuration in snmalloc-sys/Cargo.toml to enable additional build options and features by default, and introduces several new optional features for more flexible builds.
Feature configuration updates:
The default feature set now includes usewait-on-address, enabling this feature automatically for default builds.
New optional features:
Added several new optional features: tracing, pageid, vendored-stl, check-loads, and gwp-asan, allowing users to enable these capabilities as needed.
Original PR: SchrodingerZhu/snmalloc-rs#203